-
Notifications
You must be signed in to change notification settings - Fork 7
[ENH] Replace PFOA with 5NU7 and 1BRQ #198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can already see by the fact that so many locations are affected that something isn't quite right.
- can you add loaders
load_5nu7andload_1brqto be used in example code? - Why are you just deleting the 1GNH and PFOA loaders? I see no reason for that, add them back please.
- the function
structure_loaderfeels unnecessary. It just aliasesPDBParser.get_structure. Youcan also see that it was a bad idea since you need to use unnevessarylambdaexpressions to make current code work. I suggest: remove. - the function
mol_loaderfeels unnecssary. It just aliasesMoleculeLoader.__init__. It would be useful of allpdbstrings could be used, but currently only two can? I suggest: remove.
| "load_aptacom_x_y", | ||
| "load_csv_dataset", | ||
| "load_hf_dataset", | ||
| "load_pfoa", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like some exports get deleted - why?
|
as discussed. Please do not delete the code for PFOA. |
We had discussed in a meeting (where you were not present) where I asked everyone if PFOA is useful given it has no SEQRES or ATOM records and everyone said that we had no use for it given we cannot extract a sequence from it, hence it was removed. |
Fixes #197.
from the issue description: As discussed in the weekly on 20/11/2025 we will be replacing PFOA with 5NU7 and 1BRQ, given that is what is mostly used by the corporation partners of ecoSPECS. I am going to directly add them to the library as they are about 200kb in size.